-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make sync progress bars optional #1125
base: main
Are you sure you want to change the base?
Conversation
The goal is to have the ability for commands like `spfs run` and `spk env` to not show a progress bar when running the Syncer. A new `--no-progress-bars` flag has been added where appropriate. The Syncer type is no longer generic over the SyncReporter in order for Sync::get_syncer to be able to return a Syncer with either reporter type. The existing code was not doing anything useful with it being generic over the SyncReporter. Some `*_with_reporter` alternate functions have been added to spk-exec since these functions were hard-coded to show progress bars. Signed-off-by: J Robert Ray <jrray@jrray.org>
crates/spfs-cli/common/src/args.rs
Outdated
|
||
/// Do not display any progress bars when syncing objects. | ||
#[clap(long)] | ||
pub no_progress_bars: bool, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how would you feel about making this an enum instead of bool, eg --progress=none
, --progress=bars
so that we can more easily support other variations in the future without a mess of boolean flags?
For possible future expansion. Signed-off-by: J Robert Ray <jrray@jrray.org>
crates/spfs/src/sync.rs
Outdated
|
||
#[derive(Clone)] | ||
#[enum_dispatch::enum_dispatch(SyncReporter)] | ||
pub enum SyncReporters { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our downstream codebase relies on a custom sync reporter implementation - maybe we Box
up the trait instead of moving to an enum?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a Custom
variant to the enum.
Cut the size of sync.rs in half. Signed-off-by: J Robert Ray <jrray@jrray.org>
Add SyncReporters::Custom to have a way to set the reporter to anything that implements SyncReporter. Signed-off-by: J Robert Ray <jrray@jrray.org>
The goal is to have the ability for commands like
spfs run
andspk env
to not show a progress bar when running theSyncer
. A new--no-progress-bars
flag has been added where appropriate.The
Syncer
type is no longer generic over theSyncReporter
in order forSync::get_syncer
to be able to return aSyncer
with either reporter type. The existing code was not doing anything useful with it being generic over theSyncReporter
.Some
*_with_reporter
alternate functions have been added to spk-exec since these functions were hard-coded to show progress bars.